Fix thinking-block collapse and Safari layout shifts#2426
Conversation
Reproduced three shift mechanisms around reasoning blocks with harness probes before fixing: - When thinking outgrew the send-anchor's fill slack, its ~300px collapse at answer-start re-inflated the spacer and re-anchored the sent message — a measured 267px scroll of the view mid-read. The spacer is now monotonic within a turn (fresh turns still inflate, the composer-clearance floor still wins), so the collapse rides the controller's clamp path instead, which keeps everything below the block viewport-stable. - Safari has no native scroll anchoring (overflow-anchor unsupported), so any above-viewport shrink — a collapsing thinking block, a late image — shoved a detached reader's text by the full delta (350px in the probe; 0px on Chrome, whose native anchoring compensates). Where anchoring is unavailable, the chat glue now tracks the message at the viewport top while detached (binary search per scroll event) and restores its position after content resizes through a new attribution-safe controller adjustment. - The collapse itself was a single-frame teleport, and for the common lone-thinking case it couldn't even be animated: the answer-start branch flip destroyed the expanded component and mounted a collapsed one. ChatMessage now renders one renderUnits template for both the streaming and settled phases, keeping the lone thinking block's instance alive across the flip, and its expand/collapse is a height slide (220ms, disabled under prefers-reduced-motion). Side effect: earlier multi-block tool runs stay summarized while a later step streams instead of re-expanding flat. Two regression tests pin the fixed behaviors (in-turn collapse keeps content below stable; simulated-Safari detached reader stays put through an above-viewport collapse). WebKit itself cannot launch in this container, so Safari is simulated by forcing overflow-anchor off — the only engine difference relevant to these bugs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0148VmxBDazfhczvfE122XVW
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf5bde58f2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const el = children[found]; | ||
| this.readAnchor = { el, offset: el.getBoundingClientRect().top - containerTop }; |
There was a problem hiding this comment.
Track the in-message viewport anchor
When Safari manual anchoring is enabled and the viewport top is inside a long assistant message, content.children are only the top-level messagesEl children, so this stores the outer message wrapper. Collapsing a thinking block or loading an image earlier inside that same message leaves the wrapper's top unchanged while the visible paragraph moves, making delta stay zero and the Safari jump remain; anchor a descendant near the viewport top rather than the whole message wrapper.
Useful? React with 👍 / 👎.
The Safari manual-anchoring fallback tracked only top-level message wrappers. With the viewport top inside a long assistant message, a thinking block collapsing (or an image loading) above the reading position within that same message leaves the wrapper's top unchanged — delta reads zero and the jump survives. Descend from the straddling wrapper to the deepest element at the viewport top (linear scans below the top level, where children are few and can sit out of flow), which is what native scroll anchoring does. Regression test covers the in-message collapse. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0148VmxBDazfhczvfE122XVW
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5057c0c767
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| private compensateReadAnchor() { | ||
| if (!this.manualAnchoring || this.state.pinned || !this.container) return; | ||
| const anchor = this.readAnchor; | ||
| if (!anchor?.el.isConnected) return; |
There was a problem hiding this comment.
Re-anchor when the tracked node is replaced
In Safari/manual anchoring, if the anchor is one of the descendants selected above and a markdown worker result or streaming {@html} update in MarkdownRenderer.svelte replaces that node while the user is detached, isConnected becomes false. This early return leaves the stale anchor in place, so the resize that caused the replacement and any subsequent above-viewport resize are not compensated until the next user scroll, reintroducing the jump for rendered/streaming markdown; re-resolve the viewport anchor when the stored element is gone.
Useful? React with 👍 / 👎.
Markdown worker swaps and streaming re-renders replace exactly the deep paragraph nodes the manual anchor tracks. A dead node made compensation early-return, silencing it until the next user scroll — reintroducing the Safari jump for rendered/streaming markdown. The anchor is now a deepest-first ancestor chain: compensation falls back to the nearest surviving ancestor (still covering every shift above it) and re-resolves a fresh deep anchor after every pass, so a replacement can never leave a stale chain behind. Regression test replaces the anchored node in the same pass as an above-viewport shrink and checks the following pass too. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0148VmxBDazfhczvfE122XVW
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9513dc4f3a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| class="thinking-viewport mt-2 flex max-h-56 flex-col justify-end overflow-hidden md:max-h-80" | ||
| class:has-overflow={contentHeight > viewportHeight} | ||
| > | ||
| <div transition:slide={{ duration: collapseDuration, easing: cubicOut }}> |
There was a problem hiding this comment.
Keep collapsing thoughts capped during outro
When a long streaming thought finishes, loading flips to false while isOpen is still true until the effect closes it, so this slide wrapper is measured after the inner branch has switched from the capped .thinking-viewport to the uncapped settled prose. For reasoning longer than the 56/80 max-height viewport, the block can expand to the full reasoning height before the outro starts, producing a larger layout jump than the one this change is meant to smooth; keep the outgoing content in the capped streaming shape until the collapse is complete.
Useful? React with 👍 / 👎.
When loading flipped false, the template swapped the reasoning content from the capped streaming viewport to the uncapped settled prose in the same flush, BEFORE the auto-collapse effect flipped isOpen — so the slide-out of a long thought started from the full reasoning height (potentially thousands of px), an expand-then-collapse bounce worse than the jump the animation replaced. The open/collapse tracker now runs pre-render ($effect.pre), so the outro starts from the still-capped DOM, and the inner branch keeps the streaming shape whenever the block is closing, so no mid-outro re-render can grow the collapsing box. The regression test asserts the settled prose never appears during the collapse and the box never exceeds its streaming height (shape-based assertions: Tailwind's cap utilities don't load in the browser test environment, which is also why the pre-existing mask test fails there). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0148VmxBDazfhczvfE122XVW
…manual anchoring (#2427) * Stop the answer-start re-anchor jump: one-way spacer while streaming When a reasoning block outgrew the send-anchor's fill slack and then collapsed at answer start, the spacer re-inflated and re-anchored the sent message — visibly scrolling the reply the reader was following (~267px measured in the harness). This was the biggest reported shift left after #2423, previously attempted in #2426 (reverted). The spacer is now one-way while a response streams: on pure content resizes it only shrinks (a collapse rides the browser clamp instead, keeping everything below it viewport-stable), while turn starts, container resizes (mobile keyboard close, window resize, panel toggle) and settled conversations still recompute freely — so a post-stream collapse (manual toggle, think-only reply) is absorbed by re-inflation with constant scrollHeight, and the anchor is never frozen at a stale height when the viewport itself changes (the gap in the reverted attempt). Supporting controller fix: when a resize pass finds scrollTop already clamp-jumped to max (content shrank / container grew at the bottom), the follow re-pins instantly in the same frame instead of springing. The clamp paints nowhere (pre-paint restore) and the coalesced scroll event matches our write; a spring here both showed the bounce and — once the spacer re-inflation erased the clamp's max<lastMax signature — had the clamp's scroll event misread as user input, unpinning mid-stream. The snap fires ONLY on that clamp signature: an eager write on any resize would overwrite a user scroll pending in the same pass (ResizeObserver delivery can precede the scroll event) and then swallow their coalesced event as ours. Regression tests: mid-stream collapse keeps content below stable; post-stream collapse is zero-motion via re-inflation; container growth mid-turn re-derives the anchor (spacer grows, message returns to the anchor offset). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013T9Ld83jc8eegpSf39wgyD * Manual scroll anchoring for engines without overflow-anchor (Safari) Safari has no native scroll anchoring, so any above-viewport shrink — a collapsing thinking block, a late image, a markdown swap — shoved a detached reader's text by the full height delta (~350px measured; 0px on Chrome, whose native anchoring compensates). Re-lands the manual anchoring from #2426 (reverted): while detached, track the element at the viewport top (binary search over message wrappers, then a descent to the deepest straddling descendant, stored as an ancestor chain so markdown re-renders that replace the deep node fall back to a surviving ancestor) and restore its position after content resizes through an attribution-safe adjustBy on the controller. Feature-detected via CSS.supports("overflow-anchor", "auto"): Chrome keeps native anchoring and never attaches the listener. New since the reverted attempt: compensation is skipped when scrollTop moved since the anchor was captured. Resize passes are not ordered after pending scroll events (observed empirically in the harness), so the captured offsets can include a user scroll that hasn't been classified yet — "restoring" from that stale capture cancels the user's movement, i.e. the view fights the finger while reading during a stream. One uncompensated frame is invisible; the fight is not. This is the most plausible field failure mode of the original attempt. Tests: simulated Safari (overflow-anchor forced off) — above-viewport collapse keeps a detached reader stable; in-message descendant tracking; anchored-node replacement falls back and re-resolves; and a user scroll landing in the same pass as a shrink is never cancelled. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013T9Ld83jc8eegpSf39wgyD * Animate the thinking-block collapse instead of teleporting it At answer start the reasoning block collapsed ~300px in a single frame, and for the common lone-thinking reply it could not even be animated: ChatMessage rendered the streaming and settled phases from two separate template branches, so the flip destroyed the expanded component and mounted a fresh collapsed one. ChatMessage now renders one renderUnits template for both phases. The streaming behavior is intentionally unchanged from before — while the process phase streams, every group still renders its blocks flat and inline (unlike the reverted #2426, which summarized earlier runs mid-stream) — but the lone thinking block goes through the same branch in both phases, so its instance survives the flip and its expand/collapse is a height slide (220ms cubicOut, instant under prefers-reduced-motion). With the one-way spacer, the slide's shrink is absorbed frame-by-frame by the follow instead of landing as one clamp. Two subtleties carried from the reverted attempt: the open/collapse tracker runs pre-render ($effect.pre) so the slide-out starts from the still-capped streaming viewport rather than a one-frame bounce to the full settled height, and the inner branch keeps the capped shape whenever the block is closing so a mid-outro re-render cannot grow the collapsing box. Regression test asserts the settled prose never appears during the collapse and the box never exceeds its streaming height. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013T9Ld83jc8eegpSf39wgyD * Add project verify skill: drive chat-ui against a mock streaming backend Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013T9Ld83jc8eegpSf39wgyD --------- Co-authored-by: Claude <noreply@anthropic.com>
Follow-up to #2423, fixing the reported shifting around thinking blocks (Safari especially). Each mechanism was reproduced with harness probes before fixing:
What was shifting
overflow-anchorunsupported), so any above-viewport shrink — a collapsing thinking block, a late image — shoved a detached reader's text by the full height delta. Chrome's native anchoring compensates silently, which is why this read as Safari-specific.Fixes
CSS.supports("overflow-anchor", "auto"). While detached, the chat glue tracks the message element at the viewport top (binary search per scroll event) and restores its position after content resizes through a new attribution-safeadjustByon the controller. Chrome keeps native anchoring; Safari gets the equivalent.renderUnitstemplate across the streaming and settled phases, so the lone thinking block's component instance survives the answer-start flip, and its expand/collapse is a 220ms height slide (instant underprefers-reduced-motion). Side effect: earlier multi-block tool runs stay summarized while a later step streams instead of re-expanding flat.Testing
Two new browser-mode regression tests pin the fixed behaviors (in-turn collapse keeps content below stable; simulated-Safari detached reader stays put through an above-viewport collapse), alongside the existing 50-test scroll suite — all green with the full repo suite, svelte-check, and lint. WebKit itself cannot launch in the CI container (missing system libraries), so Safari is simulated by forcing
overflow-anchoroff in Chromium — the only engine difference relevant to this bug class. A manual pass on real Safari/iOS after deploy is recommended.🤖 Generated with Claude Code
https://claude.ai/code/session_0148VmxBDazfhczvfE122XVW
Generated by Claude Code